-
Couldn't load subscription status.
- Fork 2.4k
fix: resolve CI e2e test ETIMEDOUT errors when downloading VS Code #7583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add caching for VS Code test runtime to prevent re-downloads - Pin VS Code version to 1.101.2 for deterministic caching - Add pre-download step to isolate download failures from test failures - Configure version via VSCODE_VERSION environment variable This fixes the network timeout issues occurring when @vscode/test-electron attempts to download VS Code from Microsoft's CDN during CI runs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! This PR effectively addresses the CI e2e test timeout issues with a well-thought-out approach. The implementation using caching, version pinning, and pre-download isolation is clean and follows GitHub Actions best practices. I've left some suggestions inline that could further improve the robustness of the solution.
.github/workflows/code-qa.yml
Outdated
| path: apps/vscode-e2e/.vscode-test | ||
| key: ${{ runner.os }}-vscode-test-${{ env.VSCODE_VERSION }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-vscode-test- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache restore-keys fallback to just ${{ runner.os }}-vscode-test- could potentially restore an incompatible cache from a different VS Code version. Is this fallback truly beneficial, or might it cause issues if the cached version doesn't match the expected one? Consider whether a strict version match would be safer.
.github/workflows/code-qa.yml
Outdated
| key: ${{ runner.os }}-vscode-test-${{ env.VSCODE_VERSION }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-vscode-test- | ||
| - name: Pre-download VS Code test runtime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since network issues are often transient, would it be helpful to add retry logic here? Something like:
| - name: Pre-download VS Code test runtime | |
| - name: Pre-download VS Code test runtime | |
| working-directory: apps/vscode-e2e | |
| run: | | |
| for i in 1 2 3; do | |
| node -e " | |
| const { downloadAndUnzipVSCode } = require('@vscode/test-electron'); | |
| downloadAndUnzipVSCode({ version: process.env.VSCODE_VERSION || '1.101.2' }) | |
| .then(() => { | |
| console.log('✅ VS Code test runtime downloaded successfully'); | |
| process.exit(0); | |
| }) | |
| .catch(err => { | |
| console.error('❌ Failed to download VS Code (attempt ' + $i + '):', err); | |
| if ($i === 3) process.exit(1); | |
| }); | |
| " && break || sleep 5 | |
| done |
| working-directory: apps/vscode-e2e | ||
| run: echo "OPENROUTER_API_KEY=${{ secrets.OPENROUTER_API_KEY }}" > .env.local | ||
| - name: Set VS Code test version | ||
| run: echo "VSCODE_VERSION=1.101.2" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The VS Code version is specified both here as an environment variable and as a fallback in line 103. To ensure consistency and avoid potential mismatches, could we reference the environment variable directly in the Node script without the fallback? This would ensure a single source of truth.
| - name: Create .env.local file | ||
| working-directory: apps/vscode-e2e | ||
| run: echo "OPENROUTER_API_KEY=${{ secrets.OPENROUTER_API_KEY }}" > .env.local | ||
| - name: Set VS Code test version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment explaining why version 1.101.2 was chosen specifically. This would help future maintainers understand when and why to update it. For example:
| - name: Set VS Code test version | |
| - name: Set VS Code test version | |
| # Using a specific version instead of 'stable' to ensure deterministic caching | |
| # Update this version periodically to match the minimum supported VS Code version | |
| run: echo "VSCODE_VERSION=1.101.2" >> $GITHUB_ENV |
- Remove cache restore-keys fallback to prevent loading incompatible versions - Add retry logic (3 attempts) to handle transient network failures - Add 5-second delay between retry attempts These changes address the high-priority concerns raised by roomote to make the CI pipeline more robust against network issues while ensuring version consistency.
Problem
CI e2e tests were failing with ETIMEDOUT errors when downloading VS Code binaries from Microsoft's CDN during test runs.
Root Cause
The
@vscode/test-electronpackage downloads VS Code binaries on every CI run, which:Solution
This PR implements three key mitigations:
1. Caching VS Code Test Runtime
2. Pinned VS Code Version
VSCODE_VERSIONenvironment variable3. Pre-download Step
Changes
.github/workflows/code-qa.ymlto add caching and pre-download stepapps/vscode-e2e/src/runTest.tsto use pinned version with env var fallbackTesting
Fixes the network timeout issues that have been causing intermittent CI failures.
Important
Fixes CI e2e test timeouts by caching VS Code binaries, pinning version, and adding a pre-download step.
.github/workflows/code-qa.yml.1.101.2in.github/workflows/code-qa.ymlandrunTest.ts.VSCODE_VERSIONenvironment variable..github/workflows/code-qa.yml.runTest.tsto use pinned version with environment variable fallback.This description was created by
for d51ccb3. You can customize this summary. It will automatically update as commits are pushed.